Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (4)
modules/system/common/games/games.nix (1)
22-51: Nice! GameMode settings are now configurable 🎉This fully addresses the earlier feedback about hard-coded GameMode settings and greatly improves downstream composability.
modules/system/common/nix/polkit.nix (1)
13-18:⚠️ Potential issuePolkit rule grants blanket disk power – restrict or document
All members of the
usersgroup receive fullorg.freedesktop.udisks2.*rights (format, wipe, etc.). This broad scope is a known security foot-gun.Options:
- Restrict to a dedicated group (e.g.
storage).- Limit to the subset actually required (
filesystem-mount*).- If intentional, add a comment explaining the trade-off.
- if (action.id.indexOf("org.freedesktop.udisks2.") == 0 && - subject.isInGroup("users")) { + if (action.id.indexOf("org.freedesktop.udisks2.filesystem-mount") == 0 && + subject.isInGroup("storage")) {hosts/fern/default.nix (1)
73-73:⚠️ Potential issue
system.stateVersionset to unreleased “25.05”NixOS 25.05 is not released; evaluations will fail on stable channels.
- system.stateVersion = "25.05"; + # Use the latest released version your configuration supports. + system.stateVersion = "24.05";modules/hm/common/engine/default.nix (1)
16-18: Unfree package guard & minor style touch-up
unityhubis unfree; evaluation will break unlessnixpkgs.config.allowUnfree = trueis set higher up.
Also, the outer parentheses are superfluous – drop them for readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
flake.nix(3 hunks)hosts/fern/default.nix(2 hunks)hosts/oak/default.nix(1 hunks)modules/hm/common/browser/default.nix(1 hunks)modules/hm/common/communication/mail/default.nix(1 hunks)modules/hm/common/dev/global-tools/cli.nix(1 hunks)modules/hm/common/documentation/default.nix(1 hunks)modules/hm/common/engine/default.nix(1 hunks)modules/hm/common/utilities/kde-connect.nix(1 hunks)modules/hm/desktops/default.nix(0 hunks)modules/hm/hosts/oak/default.nix(4 hunks)modules/system/common/games/games.nix(2 hunks)modules/system/common/hardware/powersave.nix(1 hunks)modules/system/common/networks/print/print.nix(1 hunks)modules/system/common/nix/linux-cachyos.nix(2 hunks)modules/system/common/nix/polkit.nix(1 hunks)modules/system/common/security/password-manager.nix(1 hunks)
💤 Files with no reviewable changes (1)
- modules/hm/desktops/default.nix
🧰 Additional context used
🪛 GitHub Actions: Flake Check
modules/hm/common/documentation/default.nix
[error] 17-17: Syntax error, unexpected CONCAT operator at line 17, column 72. Likely caused by incorrect use of '++' operator in Nix expression.
flake.nix
[error] 95-96: Error while checking flake output 'nixosConfigurations' at nixosConfigurations.fern. The NixOS configuration evaluation failed due to syntax error in dependent module.
🔇 Additional comments (9)
modules/system/common/networks/print/print.nix (1)
15-21: Good: driver list is now user-configurableExposing
driverssolves the inflexibility flagged earlier – nice!hosts/fern/default.nix (1)
45-45: Import path looks goodSwitching to the concrete
hydenix.nixfile clarifies intent and avoids a redundant wrapper.hosts/oak/default.nix (1)
60-64:Details
✅ Verification successful
Validate the new Home-Manager desktop import
../../modules/hm/desktops/hydenix.nixreplaces the former directory wrapper.
Please double-check that the file really exists on all branches – a missing path will makenixos-rebuildfail at evaluation time.
🏁 Script executed:
#!/usr/bin/env bash # Confirm the target file is present exactly once in the repo fd hydenix.nix | grep -F 'modules/hm/desktops/hydenix.nix'Length of output: 90
Confirm Home-Manager desktop import path
The file
modules/hm/desktops/hydenix.nixis present exactly once in the repository, so the import inhosts/oak/default.nixis valid.modules/hm/common/communication/mail/default.nix (1)
21-23: Good catch reverting to an empty defaultRestoring
default = []keeps mail clients opt-in and avoids surprise installs. 👍modules/hm/hosts/oak/default.nix (2)
31-34:google-chrome/chromedriverrequireallowUnfree– double-check host-level nixpkgs config
Bothgoogle-chromeandchromedriverare unfree; ifnixpkgs.config.allowUnfree = true;isn’t set for this host, evaluation will fail.
76-79: Alphabetisation nit resolved – looks good
kde-connectnow precedesscalar; previous merge-conflict risk removed.modules/system/common/security/password-manager.nix (1)
26-30: Nice – udev + YubiKey manager packages now covered
Addingyubikey-manageralongsideyubikey-personalizationcloses the gap raised in the prior review.flake.nix (1)
5-5: Unpinned input lowers reproducibility
Tracking the moving branchnyxpkgs-unstableinvites non-deterministic builds. Pin to a commit or tag instead.-chaotic.url = "github:chaotic-cx/nyx/nyxpkgs-unstable"; +chaotic.url = "github:chaotic-cx/nyx/nyxpkgs-unstable?rev=<commit-hash>";modules/system/common/hardware/powersave.nix (1)
60-62: Verifyamdctlderivation availability
amdctlisn’t in upstream nixpkgs; evaluation will fail unless you overlay it. Double-check the package exists or switch toryzenadj/amdgpu-top.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (5)
modules/hm/common/utilities/kde-connect.nix (1)
9-16: Kebab-case option key complicates usage.Using a hyphen in the option name (
kde-connect) forces quoting when users configure it (modules.common.utilities."kde-connect".enable). Consider renaming tokdeConnectorkde_connectfor a more idiomatic and user-friendly API.modules/hm/common/browser/default.nix (1)
20-37: 🧹 Nitpick (assertive)Unfree browsers & drivers may fail to build without
allowUnfree
google-chrome,microsoft-edge,vivaldi,brave, and their drivers are all unfree. Unless the system already setsnixpkgs.config.allowUnfree = true, selecting any of those emulators will break evaluation.Consider adding:
config = { + nixpkgs.config.allowUnfree = true; home.packages = with pkgs;or at least documenting the requirement in the option descriptions.
modules/system/common/nix/polkit.nix (1)
13-20: Polkit rule still overly broad – reconsider or documentGranting every member of the
usersgroup unconditional access to the fullorg.freedesktop.udisks2.*namespace (including formatting) remains a serious attack surface. Prior feedback already highlighted this; please narrow the rule (e.g.filesystem-mount*) or dedicate a smaller group, or add an explicit comment justifying the trade-off.flake.nix (2)
5-5: Unpinnedchaoticinput reduces build reproducibility
Same concern as the earlier review: tracking the moving branchnyxpkgs-unstablemeans every rebuild can produce different results. Pin to a specific commit/tag or rely on the flake lock to keep it deterministic.
114-127:⚠️ Potential issue
rbhelper crashes withset -uwhen no arg providedWith
set -u,$1is unset when the script is invoked without arguments, so the script exits with “unbound variable” before your usage message executes. Initialisehostdefensively:- host=$1 + host=${1:-}Entire diff:
@@ - set -euo pipefail - host=$1 + set -euo pipefail + host=${1:-} @@ *) echo "Usage: rb [oak|fern|all]" >&2; exit 1 ;;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
flake.nix(3 hunks)modules/hm/common/browser/default.nix(1 hunks)modules/hm/common/dev/global-tools/cli.nix(1 hunks)modules/hm/common/documentation/default.nix(1 hunks)modules/hm/common/utilities/kde-connect.nix(1 hunks)modules/system/common/games/games.nix(2 hunks)modules/system/common/networks/print/print.nix(1 hunks)modules/system/common/nix/linux-cachyos.nix(2 hunks)modules/system/common/nix/polkit.nix(1 hunks)modules/system/common/security/password-manager.nix(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Flake Check
flake.nix
[error] 95-96: Nix flake check failed: Configuration error in 'nixosConfigurations.fern'. Exactly one of users.users.richen.isSystemUser and users.users.richen.isNormalUser must be set.
[error] 95-96: Nix flake check failed: Configuration error in 'nixosConfigurations.fern'. users.users.richen.group is unset. This used to default to nogroup, but this is unsafe. Please set users.users.richen.group and define users.groups.richen.
🔇 Additional comments (11)
modules/hm/common/utilities/kde-connect.nix (3)
1-4: Remove unused arguments: Good cleanup.The function signature now uses
{ config, lib, ... }, and the previously unusedpkgsargument has been removed, silencing linter warnings.
5-8: Simplify config access with let-binding: Approved.Assigning
cfgvia aletbinding makes downstream references concise and maintainable.
18-21: Verify NixOS service attributes exist.Please confirm that
services.kdeconnect.enableandservices.kdeconnect.indicatorare valid NixOS options (the upstream service may be namedkdeconnectd).modules/hm/common/dev/global-tools/cli.nix (1)
17-20: Correct use oflib.optionalresolves the previous list-of-lists issueThe switch from
lib.optionalstolib.optionalwith++concatenation gives a flat list and prevents the evaluation error flagged in the previous review. Looks good.modules/hm/common/documentation/default.nix (1)
16-19: Guard the unfreeonlyoffice-binderivation
onlyoffice-binis marked unfree in nixpkgs; evaluation will fail unlessnixpkgs.config.allowUnfree = true. Either:
- Enable it automatically when the user selects the editor, or
- Document the requirement in the option description.
Would you like a follow-up patch that sets
nixpkgs.config.allowUnfree = lib.elem "onlyoffice" cfg.editor;?modules/system/common/nix/polkit.nix (1)
1-2: 👍 Unused-arg warnings resolved
pkgsandinputswere dropped from the argument list; the previous linter warning is gone.modules/system/common/nix/linux-cachyos.nix (1)
10-17: Nice: separate flag for SCX serviceSplitting
enableSCXout from the main switch avoids surprising side-effects—good improvement.flake.nix (2)
72-75: Overlay wiring looks correct –inputs.deploy-rs.overlays.defaultis attached to the importednixpkgs.
131-138:checksblock now correctly top-level – flake schema satisfied
Nice fix; this resolves the earlier validation error.modules/system/common/networks/print/print.nix (1)
25-28: 🛠️ Refactor suggestion
cfg.driversis still evaluated unconditionallyBecause function arguments are evaluated before
lib.mkIfis applied, the
expressioncfg.driversis realised even whencfg.cups.enable = false.
To avoid forcing evaluation of (possibly large/unfree) derivations, wrap the
entire drivers attribute instead:- services.printing.drivers = lib.mkIf cfg.cups.enable cfg.drivers; + services.printing = lib.mkIf cfg.cups.enable { + drivers = cfg.drivers; + };This delays evaluation until CUPS is enabled.
Likely an incorrect or invalid review comment.
modules/system/common/games/games.nix (1)
64-74: 🧹 Nitpick (assertive)Minor: avoid eager evaluation inside
mkIfAs with the print module, the attrset passed to
lib.mkIfis evaluated
beforehand, socfg.gamemode.*is forced even when GameMode is disabled.
Not critical here but you can apply the same lazy pattern:- (lib.mkIf cfg.gamemode.enable { - gamemode = { - enable = true; - ... - }; - }) + (lib.mkIf cfg.gamemode.enable ( + lib.mkForce { + gamemode = { + enable = true; + ... + }; + } + ))Keeps large attrsets totally out of the evaluation when not needed.
Likely an incorrect or invalid review comment.
|
🎉 This PR is included in version 1.1.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Description
This pull request introduces significant updates to the configuration and module structure of the repository, with a focus on modularization, new feature additions, and cleanup of deprecated configurations. Key changes include the integration of
deploy-rsfor deployment management, consolidation of CLI tools and documentation modules, and removal of unused or outdated modules.Deployment Enhancements:
deploy-rsintegration inflake.nix, including deployment configurations for nodes (fern.localandoak.local) and a helper script (rb) for streamlined deployment commands. [1] [2]Modularization and Cleanup:
vercelandgraphite) into a single moduledev/global-tools/cli.nix, replacing individual modules. [1] [2] [3]okular,onlyoffice) into a centralizeddocumentationmodule. [1] [2] [3]multimedia/easyeffectsandutilities/gitkraken. [1] [2]New Features:
Configuration Adjustments:
flake.nixto use thenyxpkgs-unstablebranch forchaotic.urland addeddeploy-rsas an input. [1] [2]enableoptions in several modules (e.g.,mail,disk-usage). [1] [2]These changes improve the maintainability and scalability of the repository while introducing new capabilities for deployment and modular configuration.
Type of change
Checklist